Add plugin to redirect users to new profile plugin #3216
Conversation
Summary: We add a new `profile_redirect` plugin, called “Profile (moved)” in the UI, which teaches users how to install the dynamic plugin: ![Screenshot of the new plugin][ss] The instructions currently pull in Test PyPI sources; we’ll swap these out for real PyPI once the package is published. [ss]: https://user-images.githubusercontent.com/4317806/73811360-229f1300-478e-11ea-9341-f38d59170e3b.png Test Plan: Create a new virtualenv, install `tf-nightly`, uninstall `tb-nightly`, and run `//tensorboard/pip_package:extract_pip_package` and install the resulting wheel. Launch TensorBoard and note that the “Profile (moved)” plugin appears in the inactive plugins list. Click the “Copy to clipboard” button and execute the copied `pip install` command from the test virtualenv. Relaunch TensorBoard, and note that the dynamic profile plugin now appears and the “Profile (moved)” plugin is gone entirely. Unit tests included for the conditional loading behavior. Changing the attempted import (in `ProfileRedirectPluginLoader.load`) to a module that always can be imported (like `sys`) causes one of the tests to fail; changing it to one that can never be imported (like `zzz`) causes the other to fail. wchargin-branch: profile-redirect
|
Reviewers: @qiuminxu for anything profile-specific (wording, etc.); |
wchargin-branch: profile-redirect wchargin-source: e627adb6fbe65899b25d7d79e6166593d2268107
| </div> | ||
| </div> | ||
|
|
||
| <style include="dashboard-style"></style> |
There was a problem hiding this comment.
Omit this unless you are making use of it. Else, I believe you need to import https://github.com/tensorflow/tensorboard/blob/master/tensorboard/components/tf_dashboard_common/dashboard-style.html in rel="import".
There was a problem hiding this comment.
Yes, thanks! (I’d used this originally but decided that I didn’t need
it; removed the build dep and import, but forgot to remove this include.
Done.)
| } | ||
| } | ||
| })(); | ||
| tryCopy |
There was a problem hiding this comment.
FYI: since we are only "officially supporting" evergreen browsers, you may use async/await and make this code read better (wait... you are already using async/await :)).
async _copyInstallCommand() {
const tryCopy = async () => {
const textarea = this.$.commandTextarea;
textarea.select();
try {
await navigator.clipboard.writeText(textarea.textContent);
} catch (error) {
// Fallback approach.
if (!document.execCommand('copy')) {
return Promise.reject();
}
}
};
try {
await tryCopy();
this.$.copiedMessage.innerText = 'Copied.';
} catch (e) {
this.$.copiedMessage.innerText = 'Failed to copy to clipboard..';
}
}There was a problem hiding this comment.
Ah, that second try structure is definitely nicer. Done; thanks!
...rd/plugins/profile_redirect/tf_profile_redirect_dashboard/tf-profile-redirect-dashboard.html
Outdated
Show resolved
Hide resolved
...rd/plugins/profile_redirect/tf_profile_redirect_dashboard/tf-profile-redirect-dashboard.html
Show resolved
Hide resolved
wchargin-branch: profile-redirect wchargin-source: e3a25f6f2e337aa81576524367fdb9f619e7a04c
wchargin-branch: profile-redirect wchargin-source: e3a25f6f2e337aa81576524367fdb9f619e7a04c
| </div> | ||
| </div> | ||
|
|
||
| <style include="dashboard-style"></style> |
There was a problem hiding this comment.
Yes, thanks! (I’d used this originally but decided that I didn’t need
it; removed the build dep and import, but forgot to remove this include.
Done.)
...rd/plugins/profile_redirect/tf_profile_redirect_dashboard/tf-profile-redirect-dashboard.html
Outdated
Show resolved
Hide resolved
...rd/plugins/profile_redirect/tf_profile_redirect_dashboard/tf-profile-redirect-dashboard.html
Show resolved
Hide resolved
| } | ||
| } | ||
| })(); | ||
| tryCopy |
There was a problem hiding this comment.
Ah, that second try structure is definitely nicer. Done; thanks!
wchargin-branch: profile-redirect wchargin-source: c832dba267a4820a123e80f7d699d7eeeb4fdf6c
| properties: { | ||
| _installCommand: { | ||
| type: String, | ||
| readonly: true, |
There was a problem hiding this comment.
Good catch; thanks! Done.
wchargin-branch: profile-redirect wchargin-source: 29a94e1df6ff0579dbe49cce3bceb76308709bde
|
Tested works great, thanks William for the friendlier solution! |
|
Thanks for the review, @qiuminxu, and thanks for testing! |
Summary:
We add a new
profile_redirectplugin, called “Profile (moved)” in theUI, which teaches users how to install the dynamic plugin:
The instructions currently pull in Test PyPI sources; we’ll swap these
out for real PyPI once the package is published.
Test Plan:
Create a new virtualenv, install
tf-nightly, uninstalltb-nightly,and run
//tensorboard/pip_package:extract_pip_packageand install theresulting wheel. Launch TensorBoard and note that the “Profile (moved)”
plugin appears in the inactive plugins list. Click the “Copy to
clipboard” button and execute the copied
pip installcommand from thetest virtualenv. Relaunch TensorBoard, and note that the dynamic profile
plugin now appears and the “Profile (moved)” plugin is gone entirely.
Unit tests included for the conditional loading behavior. Changing the
attempted import (in
ProfileRedirectPluginLoader.load) to a modulethat always can be imported (like
sys) causes one of the tests tofail; changing it to one that can never be imported (like
zzz) causesthe other to fail.
wchargin-branch: profile-redirect